[SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions by yikf · Pull Request #37113 · apache/spark

yikf · 2022-07-07T06:57:33Z

What changes were proposed in this pull request?

Currently, Spark don't support url encode/decode as built-in functions, the user might use reflect instead, It's a bit of a hassle, And often these functions are useful.

This pr aims to two points as follow:

add url encode/decode as built-in function support.
tidy up url-related functions to one scala file

Why are the changes needed?

url encode/decode functions are useful

Does this PR introduce any user-facing change?

yes, add new function as built-in function

How was this patch tested?

add new tests

yikf · 2022-07-07T06:59:33Z

cc: @cloud-fan , I'm not sure why we don't support url encode\decode function as built-in function, but if we intentionally don't, i will close this title.

LuciferYang · 2022-07-07T12:29:48Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/urlExpression.scala

I think the file name should be urlExpressions

LuciferYang · 2022-07-07T12:38:25Z

@yikf Please run SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/testOnly *ExpressionsSchemaSuite" to regenerating the result file and update in this pr.

yikf · 2022-07-07T12:54:14Z

@yikf Please run SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/testOnly *ExpressionsSchemaSuite" to regenerating the result file and update in this pr.

thanks~ i will address those comment if this pr is necessary

cloud-fan · 2022-07-08T04:47:04Z

do other databases provide these functions?

yikf · 2022-07-08T07:35:06Z

do other databases provide these functions?

Both Trino & Presto have URL-related URL functions, including url_decode/url_encode, reference doc: https://trino.io/docs/current/functions/url.html, However, I can't find a function like url_decode in Hive or mysql, etc. maybe we have to use reflect function instead in hive, It's a bit of a hassle, And often these functions are useful.

cloud-fan · 2022-07-08T07:47:07Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/urlExpression.scala

We can make them extend RuntimeReplaceable, and use StaticInvoke as the real expression.

okay, please take a look when you have a time, thanks

AmplabJenkins · 2022-07-09T06:16:06Z

Can one of the admins verify this patch?

LuciferYang · 2022-07-11T08:00:02Z

@yikf Could you update the pr description because I found this pr also includes some refactoring work

yikf · 2022-07-11T09:17:01Z

@yikf Could you update the pr description because I found this pr also includes some refactoring work

done

cloud-fan · 2022-07-11T15:40:09Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/urlExpressions.scala

+// scalastyle:off line.size.limit
+@ExpressionDescription(
+  usage = """
+    _FUNC_(str) - Translates a string into {@code application/x-www-form-urlencoded} format using a specific encoding scheme.


Does {@code application/x-www-form-urlencoded} really work? Can you run DESC FUNCTION url_encode and give a screenshot?

Looks it work normally,

This means it does not work... The SQL doc can't interpret {@code ...}. Can we just use plain string?

.... into 'application/x-www-form-urlencoded' format ...

Sorry i misread this meaning, updated

cloud-fan

LGTM except for a minor comment

LuciferYang · 2022-07-12T09:33:52Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/urlExpressions.scala

+
+object UrlCodec {
+  def encode(src: UTF8String, enc: UTF8String): UTF8String = {
+    UTF8String.fromString(URLEncoder.encode(src.toString, enc.toString))


We use java.net.URLEncoder now.
I wonder if org.apache.commons.codec.net.URLCodec and java.net.URLEncoder have the same semantics?
Will org.apache.commons.codec.net.URLCodec have better performance?

I write a micro bench as follows:

def testEncode(url: String, valuesPerIteration: Int): Unit = { import org.apache.commons.codec.net.URLCodec val benchmark = new Benchmark("Test encode", valuesPerIteration, output = output) benchmark.addCase("Use java.net.URLEncoder") { _: Int => for (_ <- 0L until valuesPerIteration) { java.net.URLEncoder.encode(url, "UTF-8") } } val codec = new URLCodec() benchmark.addCase("Use org.apache.commons.codec.net.URLCodec") { _: Int => for (_ <- 0L until valuesPerIteration) { codec.encode(url) } } benchmark.run() } def testDecode(value: String, valuesPerIteration: Int): Unit = { import org.apache.commons.codec.net.URLCodec val benchmark = new Benchmark("Test decode", valuesPerIteration, output = output) benchmark.addCase("Use java.net.URLEncoder") { _: Int => for (_ <- 0L until valuesPerIteration) { java.net.URLDecoder.decode(value, "UTF-8") } } val codec = new URLCodec() benchmark.addCase("Use org.apache.commons.codec.net.URLCodec") { _: Int => for (_ <- 0L until valuesPerIteration) { codec.decode(value) } } benchmark.run() } override def runBenchmarkSuite(mainArgs: Array[String]): Unit = { val valuesPerIteration = 100000 // Test Contains testEncode("https://spark.apache.org", valuesPerIteration) testDecode("https%3A%2F%2Fspark.apache.org", valuesPerIteration) }

and run it use GA, the result as follows:

~~Java 8~~

OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 32 33 1 3.1 324.3 1.0X Use org.apache.commons.codec.net.URLCodec 29 29 0 3.4 291.4 1.1X OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 53 54 1 1.9 530.0 1.0X Use org.apache.commons.codec.net.URLCodec 37 37 0 2.7 365.5 1.5X

Java 11

OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 25 28 2 4.0 251.6 1.0X Use org.apache.commons.codec.net.URLCodec 22 25 1 4.6 219.5 1.1X OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 31 31 1 3.3 306.2 1.0X Use org.apache.commons.codec.net.URLCodec 26 29 2 3.8 260.4 1.2X

Java 17

OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 31 32 1 3.2 308.3 1.0X Use org.apache.commons.codec.net.URLCodec 26 29 2 3.8 262.6 1.2X OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 40 44 3 2.5 402.0 1.0X Use org.apache.commons.codec.net.URLCodec 31 33 1 3.2 311.0 1.3X ``` ~~ `org.apache.commons.codec.net.URLCodec` seems a little quick than `java.net.URLEncoder` from benchmark result, so should we change to use `org.apache.commons.codec.net.URLCodec` in this pr? @cloud-fan @Yikf

Sorry, the above bench test encode twice, need re-run, please ignore it

Update result:

Java 8

OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 40 41 1 2.5 404.9 1.0X Use org.apache.commons.codec.net.URLCodec 36 38 1 2.8 358.5 1.1X OpenJDK 64-Bit Server VM 1.8.0_332-b09 on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 38 39 1 2.6 379.0 1.0X Use org.apache.commons.codec.net.URLCodec 31 32 2 3.2 312.8 1.2X

Java 11

OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 26 26 0 3.9 256.5 1.0X Use org.apache.commons.codec.net.URLCodec 24 24 1 4.2 240.5 1.1X OpenJDK 64-Bit Server VM 11.0.15+10-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 22 22 0 4.6 217.5 1.0X Use org.apache.commons.codec.net.URLCodec 18 18 0 5.6 179.0 1.2X

Java 17

OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz Test encode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 29 33 2 3.4 290.1 1.0X Use org.apache.commons.codec.net.URLCodec 31 35 2 3.3 305.3 1.0X OpenJDK 64-Bit Server VM 17.0.3+7-LTS on Linux 5.13.0-1031-azure Intel(R) Xeon(R) CPU E5-2673 v4 @ 2.30GHz Test decode: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative ------------------------------------------------------------------------------------------------------------------------- Use java.net.URLEncoder 22 26 2 4.5 223.1 1.0X Use org.apache.commons.codec.net.URLCodec 25 29 2 4.0 252.3 0.9X

Seems like java.net.URLEncoder is the future (Java 17)? Assuming that Spark will get to Java 17 eventually, I'd vote for java.net.URLEncoder to avoid any breaking change when we want to switch to java.net.URLEncoder in the future.

Thanks @LuciferYang for benchmark test result, Seems like the benchmark did not diff much and for the reason that Spark will bump JDK and avoid any breaking change, java.net.URLEncoder is a better fit, what do you think @LuciferYang , @cloud-fan

agree with @cloud-fan

LuciferYang

LGTM +1

yikf · 2022-07-14T15:44:44Z

@LuciferYang @cloud-fan Any other comments please?

LuciferYang · 2022-07-15T02:29:27Z

friendly ping @cloud-fan

cloud-fan · 2022-07-15T04:31:21Z

thanks, merging to master!

yikf · 2022-07-15T04:37:53Z

thank you all

github-actions bot added the SQL label Jul 7, 2022

LuciferYang reviewed Jul 7, 2022

View reviewed changes

yikf closed this Jul 7, 2022

yikf reopened this Jul 7, 2022

cloud-fan reviewed Jul 8, 2022

View reviewed changes

Support url encode/decode function

2050ca6

yikf changed the title ~~[WIP] Supports url encode/decode function~~ [SPARK-39741][SQL] Support url encode/decode as built-in function Jul 11, 2022

yikf marked this pull request as ready for review July 11, 2022 07:50

yikf changed the title ~~[SPARK-39741][SQL] Support url encode/decode as built-in function~~ [SPARK-39741][SQL] Support url encode/decode as built-in function and tidy up url-related functions Jul 11, 2022

cloud-fan reviewed Jul 11, 2022

View reviewed changes

cloud-fan approved these changes Jul 11, 2022

View reviewed changes

fix comment

61344df

LuciferYang reviewed Jul 12, 2022

View reviewed changes

LuciferYang approved these changes Jul 12, 2022

View reviewed changes

cloud-fan closed this in e5c1b82 Jul 15, 2022

bjornjorgensen mentioned this pull request Aug 4, 2023

[SPARK-44155] Adding a dev utility to improve error messages based on LLM #41711

Closed

Comments

Conversation

yikf commented Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

yikf commented Jul 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Jul 7, 2022

Uh oh!

yikf commented Jul 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yikf commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Jul 9, 2022

Uh oh!

LuciferYang commented Jul 11, 2022

Uh oh!

yikf commented Jul 11, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang left a comment

Choose a reason for hiding this comment

Uh oh!

yikf commented Jul 14, 2022

Uh oh!

LuciferYang commented Jul 15, 2022

Uh oh!

cloud-fan commented Jul 15, 2022

Uh oh!

yikf commented Jul 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yikf commented Jul 7, 2022 •

edited

Loading

yikf commented Jul 7, 2022 •

edited

Loading

cloud-fan commented Jul 8, 2022 •

edited

Loading

yikf commented Jul 8, 2022 •

edited

Loading

LuciferYang Jul 12, 2022 •

edited

Loading